fix: infer HTTP for URL-only Codex MCP imports#85
fix: infer HTTP for URL-only Codex MCP imports#85brushax wants to merge 1 commit intoSaladDay:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Codex MCP import so URL-only entries (missing type but with url) are treated as HTTP servers instead of defaulting to stdio, addressing import failures for valid remote MCP servers.
Changes:
- Update
import_from_codex()to infertype = "http"whenurlis present and non-empty andtypeis omitted. - Add a regression test covering URL-only Codex MCP entries, including preservation of custom fields like
bearer_token_env_var.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src-tauri/src/mcp.rs |
Adjust Codex import type inference to treat URL-only entries as HTTP. |
src-tauri/tests/import_export_sync.rs |
Add regression test ensuring URL-only Codex entries import as HTTP and keep custom fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let typ = entry_tbl | ||
| .get("type") | ||
| .and_then(|v| v.as_str()) | ||
| .or_else(|| { | ||
| entry_tbl | ||
| .get("url") | ||
| .and_then(|v| v.as_str()) | ||
| .filter(|url| !url.trim().is_empty()) | ||
| .map(|_| "http") | ||
| }) | ||
| .unwrap_or("stdio"); |
There was a problem hiding this comment.
The new inference uses .get("type").and_then(|v| v.as_str()).or_else(...), which will infer http not only when type is absent, but also when type is present but not a string (e.g. type = 1). That diverges from the PR intent (“type is absent”) and can silently import misconfigured entries. Consider checking entry_tbl.contains_key("type") first (or filtering out non-string/empty values as a validation error) so inference only happens when the key is truly missing.
| let typ = entry_tbl | |
| .get("type") | |
| .and_then(|v| v.as_str()) | |
| .or_else(|| { | |
| entry_tbl | |
| .get("url") | |
| .and_then(|v| v.as_str()) | |
| .filter(|url| !url.trim().is_empty()) | |
| .map(|_| "http") | |
| }) | |
| .unwrap_or("stdio"); | |
| let typ = if entry_tbl.contains_key("type") { | |
| // 如果显式提供了 type，则仅使用该字段（缺失或类型不匹配时回退到 stdio）， | |
| // 不再根据 url 推断为 http，避免静默掩盖配置错误。 | |
| entry_tbl | |
| .get("type") | |
| .and_then(|v| v.as_str()) | |
| .unwrap_or("stdio") | |
| } else { | |
| // 仅在完全省略 type 时，才允许通过非空 url 推断为 http。 | |
| entry_tbl | |
| .get("url") | |
| .and_then(|v| v.as_str()) | |
| .filter(|url| !url.trim().is_empty()) | |
| .map(|_| "http") | |
| .unwrap_or("stdio") | |
| }; |
| // Codex 的远程 MCP 可以只写 `url`，不显式提供 `type`。 | ||
| // 这种情况下应按 HTTP 服务器导入，而不是回退到 stdio。 | ||
| let typ = entry_tbl | ||
| .get("type") | ||
| .and_then(|v| v.as_str()) | ||
| .or_else(|| { | ||
| entry_tbl | ||
| .get("url") | ||
| .and_then(|v| v.as_str()) | ||
| .filter(|url| !url.trim().is_empty()) | ||
| .map(|_| "http") | ||
| }) | ||
| .unwrap_or("stdio"); |
There was a problem hiding this comment.
Note: if typ ends up unknown (including the case where type is an empty string), the later match typ { _ => { ... return changed; } } returns from the import_servers_tbl closure, which stops processing remaining servers in the same table. Since the intent is to “skip” invalid items, this should likely continue the per-item loop instead, and/or filter out empty/whitespace type values up front to avoid hitting the unknown-type path.
Summary
typeis omitted buturlis presenttypeand stdio entriesbearer_token_env_var)Root cause
import_from_codex()defaulted missingtypetostdiounconditionally. Valid Codex URL-based entries without an explicittypewere therefore validated as stdio and rejected due to missingcommand.Fix
In Codex import, infer
httpwhen:typeis absenturlexists and is non-emptyOtherwise keep the existing fallback to
stdio.Validation
cargo fmt --manifest-path src-tauri/Cargo.toml --all --checkcargo test --test import_export_sync import_from_codex --manifest-path src-tauri/Cargo.tomlcargo test --test import_export_sync --manifest-path src-tauri/Cargo.tomlCloses #84